Handle pg_dump restrict commands in local restore#611
Handle pg_dump restrict commands in local restore#611
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the local database restore flow to be compatible with newer pg_dump output by removing psql-only \restrict/\unrestrict meta-commands during staging, and fixes the restore result construction while adding CLI test coverage for the new sanitization behavior.
Changes:
- Strip
\restrictand\unrestrictlines (in addition to existing role-dependent statements) from staged SQL before invokingpsql. - Ensure
LocalDbRestoreResultconstruction includes the staged SQL path (sql_file). - Extend CLI restore test to cover dumps that include the new meta-commands.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
cli/db_restore.py |
Adds sanitization for pg_dump meta-commands and fixes LocalDbRestoreResult construction. |
tests/test_cli_commands.py |
Updates restore CLI test input to include \restrict/\unrestrict and asserts they’re removed. |
You can also share your feedback on Copilot code review. Take the survey.
| return LocalDbRestoreResult( | ||
| sql_file=staged_sql_file, |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20aafdd21d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| matches_psql_meta = any( | ||
| pattern.search(line) for pattern in PSQL_META_COMMAND_PATTERNS | ||
| ) | ||
| if matches_role_sql or matches_psql_meta: |
There was a problem hiding this comment.
Do not strip pg_dump's
\restrict safety guard
PostgreSQL 17.6 added \restrict specifically to block injected psql meta-commands during plain-text restores (see the 17.6 release notes for CVE-2025-8714, and app-psql's note that restricted mode only allows \unrestrict). Because this sanitizer removes those lines and then still feeds the file to psql -f, any dump restored from GCS or another compromised source can once again execute meta-commands such as \! on the operator's machine. For older local clients we should fail fast or require a newer psql, not silently discard the protection.
Useful? React with 👍 / 👎.
| return LocalDbRestoreResult( | ||
| sql_file=staged_sql_file, |
There was a problem hiding this comment.
Avoid returning a staged SQL path that is already deleted
result.sql_file now points at staged_sql_file, but _stage_restore_source() creates that file inside a TemporaryDirectory() and removes it before restore_local_db_from_sql() returns. Any caller that tries to inspect or reuse result.sql_file after a successful restore will immediately hit a missing file, so the new field does not actually provide the staged SQL path advertised by LocalDbRestoreResult.
Useful? React with 👍 / 👎.
Summary
\restrictand\unrestrictmeta-commands from staged restore SQL before invoking local psqlTesting